@tidely great work! your PR is more simplistic (not introducting logging) and as you mention not backwards compatible as the id is not optional. i think i'll add a test like you did so it covers both types of possible inputs.
Maybe we should add a issue and link it to both PR's so zed-maintainers can decide how to proceed with that.

May i ask how - if you did not have logging - you discovered that? I must admit that i had a really hard time to figure out what was going on.

not introducting logging

I think the error already gets logged in high level code after the error bubbles up. Not sure how useful the errors are anyway, I think it might be worth exploring using the serde_path_to_error crate already used in other places in the Zed codebase. We should definitely merge our efforts on this though.

Maybe we should add a issue and link it to both PR's so zed-maintainers can decide how to proceed with that.

Sounds good!

May i ask how - if you did not have logging - you discovered that?

I compared the request schema used in tests to the one returned by curling the newest ollama version, and then I did some digging in the Ollama repo

@tidely note sure how to effectively continue. i think the issue you referenced is good.

i focused on adding logging (debug&trace) for the ollama module which is great when you want to see how the agent interacts but effectively this is not really related to the issue.
So my PR should be ollama: add logging while yours should just add the optional fields and both tests in order to be backwards compatible.

As per your recommendation @js-0s, I've made the PR backwards compatible with older Ollama versions and added a test for this behavior.

Initially I thought we should just embrace the new response format and ditch older Ollama installations, as we haven't really concerned ourselves with Ollama backwards compatibility before either. It would've allowed us to remove some custom logic we use for our own internal tool calling identifiers and brought Ollama more in unison with other providers. But I figured some well documented TODO's and comments will allows us to strip those bits away later as well.

Thanks for your input here and your efforts with your own PR as well!